-
Notifications
You must be signed in to change notification settings - Fork 321
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support parse raid type for mdstat #505
base: master
Are you sure you want to change the base?
Conversation
This needs a DCO sign-off. You can use |
@SuperQ Sorry that I forgot the sign-off in later commits😂. And now it's added. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the original checking for (...)
in the word after active/inactive was better.
mdstat.go
Outdated
@@ -165,6 +175,10 @@ func parseMDStat(mdStatData []byte) ([]MDStat, error) { | |||
return mdStats, nil | |||
} | |||
|
|||
func isRaidType(mdType string) bool { | |||
return strings.HasPrefix(mdType, "raid") || mdType == "linear" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I don't think this is going to work. Like I said, there are a whole bunch of other types other than raid...
. Since this can change depending on the kernel/mdadm version. We can't support an exhaustive list here.
mdstat.go
Outdated
@@ -95,6 +97,13 @@ func parseMDStat(mdStatData []byte) ([]MDStat, error) { | |||
mdName := deviceFields[0] // mdx | |||
state := deviceFields[2] // active or inactive | |||
|
|||
mdType := "unknown" | |||
if len(deviceFields) > 3 && isRaidType(deviceFields[3]) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you move this to after if len(lines) <= i+3 {
you can reduce the word checking to just the check for (...)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, maybe this whole piece should be moved to the evalStatusLine()
function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
len(lines) <= i+3
is the guard for lines not the fields in first line, so the move won't be helpful
@SuperQ There's a case that I cannot check
If we only check parentheses, the type will be But maybe we can assume the type does not contain any special characters? |
@SuperQ I changed the code for type checking. It passes all current tests, and I hope it can work forever. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we only get a type if state is active
. Maybe check that first before checking the next word or two.
@SuperQ I considered this before. But there is another case...
We can ignore the type for this, but it's here anyway and it may be useful in some case. |
Ahh, crap, yea, that's a problem. |
// check if a string's format is like the mdType | ||
// Rule 1: mdType should not be like (...) | ||
// Rule 2: mdType should not be like sda[0] | ||
func isRaidType(mdType string) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ewww hah..
There must be a better way. Is there an explicit way to query the md type? E.g by reasing another procfs file?
@SuperQ @discordianfish Because of the lack of some knowledge, let's keep this open and see if anyone can help. |
@robbat2 Sure, I will update this later this day! |
Signed-off-by: Singee <[email protected]>
Signed-off-by: Singee <[email protected]>
Signed-off-by: Singee <[email protected]>
Signed-off-by: Singee <[email protected]>
@robbat2 I have added the Sign-Off |
Thank you; rebased my PR now |
For something like
We can also get the raid type from /proc/mdstat